Skip to content

Conversation

@makortel
Copy link
Contributor

@makortel makortel commented Aug 29, 2025

PR description:

Testing ROOT PR on an option to disable header parsing during TClass::GetClass() call (root-project/root#18402) it was noticed the type alias gets listed in the rootmap file only if the alias is requested before the real type (see root-project/root#19705 for more details).

In the mean time, having the type alias in the rootmap file is necessary to avoid header parsing for the execution of the read rules that use the type alias names, e.g.

rule.fSource = type + "::Layout layout_;";

, and therefore it seemed worthwhile to change the dictionaries with PortableHost{Collection,Object}.

In the present state this PR demonstrates what the impact would be for the PortableTestObjects. If deemed viable, the next steps would be to update DataFormats/Portable README and scripts, and then update all the other classes_def.xml files that declare these portable data products.

Resolves cms-sw/framework-team#1542

PR validation:

The testHeterogeneousCoreAlpakaTestWriteReadSerialSync unit test succeeds with the build of cms-sw/root#222 (comment). The resulting rootmap contains the ::Product, ::Layout, and ::Implementation type aliases that the read rules use.

…heir aliased types in classes_def.xml

In ROOT presently the type alias gets listed in the rootmap file only
if the alias is requested before the real type. Having the type alias
in the rootmap file is necessary to avoid header parsing for the
execution of the read rules that use the type alias names.
@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 29, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel for master.

It involves the following packages:

  • DataFormats/PortableTestObjects (heterogeneous)

@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks.
@missirol, @mmusich, @rovere this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-556bb6/47912/summary.html
COMMIT: cb4f965
CMSSW: CMSSW_15_1_X_2025-08-29-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/48824/47912/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4115479
  • DQMHistoTests: Total failures: 29
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4115430
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 215 log files, 184 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

Milestone for this pull request has been moved to CMSSW_16_0_X. Please open a backport if it should also go in to CMSSW_15_1_X.

@makortel
Copy link
Contributor Author

@fwyzard Given

regardless of #49458 we should probably revive this PR and go systematically through all dictionary definitions of Portable* types (and update documentation).

I have a vague recollection though there was some detail I was not happy with, but I can't remember what it was.

@fwyzard
Copy link
Contributor

fwyzard commented Nov 25, 2025

I must admit I actually didn't remember about this PR :-(

@fwyzard
Copy link
Contributor

fwyzard commented Nov 25, 2025

Is there any way we can make the read rules themselves more robust?

@fwyzard
Copy link
Contributor

fwyzard commented Nov 25, 2025

For example, something I noticed earlier today is that for the (deprecated) cases where a class inherits from a PortableCollection, the read rule refers to the portable collection, not to the derived class.
I don't know what is the correct behaviour, but I found the current one confusing...

@makortel
Copy link
Contributor Author

Is there any way we can make the read rules themselves more robust?

We'd need help from ROOT @pcanal @vepadulano

For example, something I noticed earlier today is that for the (deprecated) cases where a class inherits from a PortableCollection, the read rule refers to the portable collection, not to the derived class.
I don't know what is the correct behaviour, but I found the current one confusing...

To my understanding of ROOT read rules (that could be wrong) I believe that case would work properly. ROOT uses the PortableCollection read rule only for the base class part, and the rest of the deriving class gets (de)serialized according its own structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As a workaround add PortableHost* type aliases to their classes_def.xml files.

3 participants